Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop writting application metadata in muxed files #3843

Merged
merged 3 commits into from
Jul 6, 2020

Conversation

kapodamy
Copy link
Contributor

@kapodamy kapodamy commented Jul 4, 2020

What is it?

[✔] Bug fix (user facing)
[✖] Feature (user facing)
[✖] Code base improvement (dev facing)
[✔] Meta improvement to the project (dev facing)

Description of the changes in your PR

By default an encoder/muxer writes a metadata called "writing and/or encoder application" for example Lavf5 8.38.100 in FFMPEG, NewPipe does the same and this PR will remove it.

The following format muxers are afected:

  • WebM
  • Mp4
  • OGG

Also, fixes most of checkstyle's issues (excepts those large method lines)

Fixes the following issue(s)

resolves #3839

Testing apk

app-debug.zip

Final notes

In non-muxed files original google's metadata are present:

Agreement

[✔] I carefully read the contribution guidelines and agree to them.

kapodamy added 2 commits July 3, 2020 02:07
* All muxers (mp4, webm and ogg) are affected
* solve some checkstyle's errors (building was blocked)

Mp4FromDashWriter:
* drop "writing application"
* drop "handler name"

OggFromWebMWriter:
* drop "writing application" for OPUS and VORBIS header

WebMWriter:
* Drop "Muxing application"
* Drop "Writing application"
@TobiGr TobiGr added the downloader Issue is related to the downloader label Jul 4, 2020
@wb9688 wb9688 added this to the 0.19.6 milestone Jul 5, 2020
@wb9688
Copy link
Contributor

wb9688 commented Jul 5, 2020

The code looks OK (though I don't like trailing commas e.g. here). I assume all the changed values are changed offsets, is that correct?

Btw how did you get all your knowledge about codecs and stuff?

Also, fixes some checkstyle's issues (excepts those large method lines)

If you fix Checkstyle issues, please remove them from the suppressions.

@kapodamy
Copy link
Contributor Author

kapodamy commented Jul 6, 2020

  1. yes, because some offsets are hardcoded
  2. from nothing (literally), i take look at the specifications of each format, but most of the time it was RE

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you ❤️

@TobiGr TobiGr merged commit af098aa into TeamNewPipe:dev Jul 6, 2020
This was referenced Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
downloader Issue is related to the downloader
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please make your metadata OPTIONAL!
3 participants